Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Demonstrate expected behaviour for QueryBuilder::setLocale #607

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

uwej711
Copy link
Contributor

@uwej711 uwej711 commented Mar 6, 2015

Failing test for issue #605

I would expect setLocale on the QueryBuilder would return the found document in the specified locale

@dantleech
Copy link
Contributor

I will try and have a look at this tomorrow. This is surely meant to work but couldn't find any existing tests, very odd.

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 8, 2015

Maybe adding a doLoadTranslation here would be a way to do it:
https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/Query/Query.php#L183

@dbu
Copy link
Member

dbu commented Mar 9, 2015 via email

@dantleech
Copy link
Contributor

It seems that it would be ideal to add a $locale argument to the loadDocumentsByPhpcrQuery, but that cascades then to findMany() (which can then pass the locale hint to the unit of work getOrCreateDocuments method).

But by this logic we should add the $locale argument to all of the document manager methods (including persist). If that is a good idea or not, I don't know, but I guess its out of the scope here.

Another option would be to add translation aware methods, e.g. getTranslatedDocumentsByPhpcrQuery, findManyTranslated.

But I vote for @dbu s suggestion for now.. and we could create an issue to solve this in 2.0

@dbu
Copy link
Member

dbu commented Mar 14, 2015

the only other option i see would be to remove the possiblility to specify the locale in the query builder. that would be more consistent with the rest. then we would say whatever you do, you need yourself to set the default locale if you want to change something about what the default locale is.

i think we can say that changing the default locale for a single call is out of scope of phpcr-odm, and you just need to change the default if you need this.

or we could offer some sort of wrapper-system where you can say $dm->defaultLocale('de')->find... defaultLocale() would return a dm decorator that changes the locale on the fly and changes it back.

@dantleech
Copy link
Contributor

Opened issue #623 to discuss how to handle this

@lsmith77 lsmith77 changed the title Demonstrate expected behaviour for QueryBuilder::setLocale [RFC] Demonstrate expected behaviour for QueryBuilder::setLocale May 1, 2015
@lsmith77 lsmith77 changed the title [RFC] Demonstrate expected behaviour for QueryBuilder::setLocale Demonstrate expected behaviour for QueryBuilder::setLocale May 1, 2015
@lsmith77 lsmith77 added ready and removed in progress labels May 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants